Centaur Review

Found 4 issue(s) (2 warning).

packages/client/src/connection.ts

Correct fix for a real iOS bug (stale readyState after background). Main concern is that defuseOldWs doesn't close() the old socket, leaving orphaned connections on desktop tab switches — adding ws.close() before nulling would be a clean improvement. Missing tests for a behavior change in a TDD project.

• 🟡 regressions (L339): defuseOldWs() detaches handlers and drops the reference but never calls ws.close(). Previously this path only ran for dead sockets; now it runs unconditionally on every foreground return — including desktop tab switches where the socket is still healthy. The orphaned open socket stays alive on both client and server until GC or timeout, briefly creating two server-side connections per client. Consider calling ws.close() before nulling the reference (the detached onclose=null already prevents the handler from interfering). [fixable]
• 🔵 regressions (L337): On desktop browsers, every visibilitychange→visible (any tab switch back) now tears down a healthy WebSocket and forces a full reconnect + hello/welcome handshake. This causes a brief _close→_open state transition on every tab switch, which may produce UI flicker in downstream listeners. The tradeoff is valid for iOS, but you could gate the unconditional teardown behind a platform check or at least skip it when the socket has recently proven alive (e.g., received a message within the last heartbeat interval). [fixable]
• 🟡 missing_tests: The @mitzo/client package has zero test files. This PR changes a behavioral contract (conditional reconnect → unconditional teardown) with implications for both iOS and desktop. Given the project's TDD requirement, at minimum the new behavior should have tests: (1) checkAndReconnect tears down an OPEN socket and creates a new one, (2) reconnectTimer guard prevents double-reconnect, (3) sends during reconnect window queue in pendingSends. Pre-existing gap, but this change makes it more consequential. [fixable]
• 🔵 style (L268): The heartbeat (line 268) still conditionally checks readyState before reconnecting, while checkAndReconnect now unconditionally tears down. The inconsistency is intentional (heartbeat fires every 5s in foreground, so it should only catch dead sockets), but a brief inline comment noting why the heartbeat keeps its guard while checkAndReconnect doesn't would prevent a future reader from "fixing" the heartbeat to match. [fixable]